Skip to content

perf: optimize Array.interleave implementation#7536

Open
ElectricalBoy wants to merge 3 commits into
mainfrom
array-interleave-optimize
Open

perf: optimize Array.interleave implementation#7536
ElectricalBoy wants to merge 3 commits into
mainfrom
array-interleave-optimize

Conversation

@ElectricalBoy
Copy link
Copy Markdown
Collaborator

How did you test this change?

included test

Performance Test Results

Array.interleave(Array.range(1, n --[[ n is a positive integer ]]), ' ')
$n = 10^4$ $n=5\times10^4$ $n=10^5$ $n=2.5\times10^5$ $n=5\times10^5$
Runtime (Current) 0.028 s 0.074 s 0.137 s 0.326 s Out of memory
Memory (Current) 2.748 MB 9.734 MB 18.728 MB 41.517 MB Out of memory
Runtime (PR) 0.020 s 0.046 s 0.050 s 0.100 s 0.194 s
Memory (PR) 1.840 MB 4.199 MB 7.345 MB 13.637 MB 26.219 MB

@ElectricalBoy ElectricalBoy requested review from a team as code owners May 21, 2026 03:52
@ElectricalBoy ElectricalBoy force-pushed the array-interleave-optimize branch from e614c20 to 5f50634 Compare May 21, 2026 03:58
Copy link
Copy Markdown
Collaborator

@hjpalpha hjpalpha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

@mbergen
Copy link
Copy Markdown
Collaborator

mbergen commented May 21, 2026

This removes the flattening aspect (which arguably shouldn't have been part of it in the first place), is there anything depending on that?

@ElectricalBoy
Copy link
Copy Markdown
Collaborator Author

ElectricalBoy commented May 21, 2026

This removes the flattening aspect (which arguably shouldn't have been part of it in the first place), is there anything depending on that?

not a factor as shown with 9862ae2

Array.flatten flattens one level and the mapping function of the original impl was adding a level so the flattening aspect cancelled out anyway

if index == size then
return {element}
end
return {element, x}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants